Skip to content

fix(compilers/openapi): stop rounding raw-preserved numbers - #244

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-raw-number-fidelity
Open

fix(compilers/openapi): stop rounding raw-preserved numbers#244
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-raw-number-fidelity

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

annotation.RawFromNode decoded a YAML node into any and re-marshalled it as JSON. yaml.v3 resolves an out-of-int64 integer and every decimal into float64, so the one channel whose documented promise is verbatim preservation quietly rewrote its numbers:

source preserved as (before)
12345678901234567890123 1.2345678901234568e+22
1.000000000000000000001 1
-9223372036854775809 -9223372036854776000
1.10 1.1

One conversion backs every raw-preservation site, so all four were affected: x-* extensions, oneOf/anyOf sibling preservation, the §4.7 validation-only carve-out, and tuple items-after-prefix. #23 fixed the same class of bug in the Value/BigVal channel; this is the raw channel's version of it.

The fix renders the node tree directly, taking numeric scalars from their source text via value.NumericLiteral — the helper that already backs the Value channel, and that resolves YAML's integer bases, so 0o17 still reads 15 rather than 17.

Walking the tree means the structural work the decode used to do has to be done here, so merge-key (<<) expansion, duplicate-key rejection, non-string-key refusal, alias following and sorted object keys are all reproduced deliberately and pinned by tests.

ir.UnmodeledEntry.Value's doc comment described the rounding as intended behaviour ("normalizes … number spelling"), so it is corrected to say what now holds.

Behaviour changes beyond the rounding

Deliberately out of scope

Timestamps still normalize to RFC 3339 and !!binary still carries decoded bytes rather than its base64 text, which costs ill-formed UTF-8 its identity to U+FFFD. Those are a different mechanism from the float64 rounding and are filed as #242, noted in the code and in the Value doc comment rather than swept in here.

Test plan

  • TestRawFromNode_KeepsNumericLiteralsExact — the regression, over the spellings the old conversion rewrote, including a significant trailing zero and both integer boundaries.
  • TestRawFromNode_ResolvesYAMLIntegerBases — the half that must not become verbatim, so the fix cannot be "read the source text" applied too far.
  • TestRawFromNode_PreservesMergeAndOrderingSemantics and TestRawFromNode_RefusesWhatJSONCannotName — the structural behaviour the walk had to reimplement, and the refusals compilers/openapi: a failed raw conversion preserves nothing while its diagnostic claims otherwise #144 depends on.
  • TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers — the equivalence oracle. The old conversion is kept in the test file, and the claim is that rounding the new output through float64 reproduces it. Reading two implementations cannot show they agree; this can.
  • Bounds: an alias cycle, a multiplicative alias chain, and a << chain that recurses without passing back through the node walk.
  • testdata/conformance/openapi/numeric-precision.yaml gains a preserved property carrying high-precision numbers in an extension and under not. The corpus had no case with a number float64 cannot hold, which is why nothing caught this.

Verified by mutation rather than by inspection — each of these was planted and confirmed to redden: reverting to the float64 decode, inverting merge precedence, dropping the duplicate-key check, dropping key sorting, accepting non-string keys, and removing the bounds (which turns a clean refusal into a stack overflow). Restoring the original bug reddens the conformance case on all four preserved values, so the fixture is load-bearing rather than decorative.

Full gate green: gofmt, go vet, golangci-lint, go build, and ./scripts/check-coverage.sh at 100% of 4469 statements.

Fixes #32

RawFromNode decoded a YAML node into `any` and re-marshalled it. yaml.v3
resolves an out-of-int64 integer and every decimal into float64, so the
one channel whose documented promise is verbatim preservation quietly
rewrote its numbers: a 23-digit extension value came back as
1.2345678901234568e+22, and 1.000000000000000000001 came back as 1.

That reached every raw-preservation site — x-* extensions, oneOf/anyOf
sibling preservation, the validation-only carve-out, and tuple
items-after-prefix — because all four share this one conversion.

Render the node tree directly instead, taking numeric scalars from their
source text through value.NumericLiteral, which already backs the
Value/BigVal channel and resolves YAML's integer bases (0o17 is still
15, not 17). Walking the tree means reimplementing what the decode did
structurally, so merge-key expansion, duplicate-key rejection,
non-string-key refusal, alias following and sorted object keys are all
kept, and pinned by tests.

Two behaviour changes beyond the rounding. An explicitly tagged huge
integer (`!!int 12345678901234567890123`) used to be dropped with a
diagnostic and is now preserved. A literal whose rendered form is not
JSON is refused rather than spliced into the document, which keeps the
open NewBigVal binary-exponent gap (#45) out of the IR without settling
it here.

Timestamps and !!binary scalars are still rewritten rather than kept
verbatim. That is a different mechanism from the rounding and is filed
as #242 rather than swept in.

Fixes #32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openapi: raw preservation routes numbers through float64, corrupting extensions and preserved keywords

1 participant